Skip to content

fix(select): remove item focus style when there are no selected items only at ionic mode #30750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Oct 30, 2025
Merged

fix(select): remove item focus style when there are no selected items only at ionic mode #30750

merged 12 commits into from
Oct 30, 2025
+17 −1

Conversation

@joselrio
Copy link
Contributor

Issue number: resolves #

This PR introduces improvements to the visual focus styling of Ionic items when inside a select modal.

What is the current behavior?

  • The CSS variables --background-focused and --background-focused-opacity were missing from item.ionic.scss, which resulted in the native default outline focus style being applied to focused items.
  • When no item is selected, the focus style is currently applied to the first list item by default, which we intend to change.

What is the new behavior?

  • Added missing focus css variables
  • Hide the default focus style when there is no item selected

NOTE

  • This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@joselrio joselrio requested a review from a team as a code owner October 24, 2025 14:34
@joselrio joselrio requested a review from thetaPC October 24, 2025 14:34
@vercel
Copy link

vercel bot commented Oct 24, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ionic-framework Ready Ready Preview Comment Oct 29, 2025 5:16pm

@github-actions github-actions bot added the package: core @ionic/core package label Oct 24, 2025
thetaPC
thetaPC requested changes Oct 28, 2025
View reviewed changes
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few things as well:

  1. Checks are failing. Please have those passing before asking for a review.
  2. This is causing a regression. Notice how there's a border radius when opening a select popover:
next ROU-12235
https://github.com/user-attachments/assets/a156e4da-2fbc-472b-9b33-b9aaa85db78c https://github.com/user-attachments/assets/ff9d844d-ddbf-42b4-baf0-632ac8cf6053

I haven't checked the other interfaces but I assume that they have the same issue. Please check.

  1. What's the ticket to add in the work for "This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling."? Because the options are no longer showing the focus rings when navigating with keyboard.

@joselrio
Copy link
Contributor Author

joselrio commented Oct 29, 2025 •
edited
Loading

3. What's the ticket to add in the work for "This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling."? Because the options are no longer showing the focus rings when navigating with keyboard.

Basically we must add this behaviour just when keyboard navigation is used to navigate through the elements, before this implementation it was defined on top of tap navigation which causes this visual issue, now focus style is missing when there are no pre-selected option when keyboard navigation is in use.

thetaPC
thetaPC requested changes Oct 29, 2025
View reviewed changes
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that we should add a test under select-modal to capture the focus states when an option is selected and when it's not.

@thetaPC
Copy link
Contributor

thetaPC commented Oct 29, 2025

  1. What's the ticket to add in the work for "This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling."? Because the options are no longer showing the focus rings when navigating with keyboard.

Basically we must add this behaviour just when keyboard navigation is used to navigate through the elements, before this implementation it was defined on top of tap navigation which causes this visual issue, now focus style is missing when there are no pre-selected option when keyboard navigation is in use.

@joselrio I understand that the behavior needs to be added for keyboard. What's the Jira ticket that implements it?

@joselrio
Copy link
Contributor Author

Just realized that we should add a test under select-modal to capture the focus states when an option is selected and when it's not.

I agree, but let's do it under the context of the task I mentioned before since focus should be specially tackled under that context ;)

@joselrio
Copy link
Contributor Author

  1. What's the ticket to add in the work for "This change will require an additional interaction to observe the focus behavior when navigating through keyboard, since tap-based navigation does not rely on focus styling."? Because the options are no longer showing the focus rings when navigating with keyboard.

Basically we must add this behaviour just when keyboard navigation is used to navigate through the elements, before this implementation it was defined on top of tap navigation which causes this visual issue, now focus style is missing when there are no pre-selected option when keyboard navigation is in use.

@joselrio I understand that the behavior needs to be added for keyboard. What's the Jira ticket that implements it?

Task do not exist yet, but it will be created since we already aligned with @jessicamendesOS and @gnbm about it.

@joselrio joselrio requested a review from thetaPC October 30, 2025 10:54
thetaPC
thetaPC approved these changes Oct 30, 2025
View reviewed changes
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joselrio joselrio merged commit 28caeff into next Oct 30, 2025
49 checks passed
@joselrio joselrio deleted the ROU-12235 branch October 30, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thetaPC thetaPC thetaPC approved these changes

Assignees

No one assigned

Labels

package: core @ionic/core package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joselrio @thetaPC @Ionitron